Repository navigation
Conversation
Member
|
Few thoughts after reading OP (didn't read code):
|
ElectricalBoy
added
the
qol
http-mitm-proxy has had no release since 2023-11 and 92 open issues; the IPv4-loopback workaround in proxy.js existed only to route around one of its bugs. Drop it and reuse scripts/proxy_lp.py, the mitmproxy addon already on main, so interception logic lives in one reviewed place instead of being duplicated in classify.js. The node side keeps everything that made this worth having: throwaway browser profile (so no CA is installed anywhere), PAC scoping, browser detection, and the rebuild watcher. Only the intercept moves. proxy_lp.py absorbs classify.js's stricter matching — subdomains, an actual /commons/load.php path check, and parsed query params rather than substring tests on the whole URL — plus no-store and passthrough when nothing is built yet. Its reload half stays inert unless LP_DEV_RELOAD=1, so running it standalone injects nothing. Reloads no longer clobber wiki previews. A css-only rebuild swaps the stylesheet in place, and a js rebuild holds its reload back while an edit form or unsaved textarea is live, retrying once that clears. --no-reload opts out entirely. Rebuilds are handed over via a build-id marker file that the proxy reads without consuming, so every open tab sees every rebuild.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new npm run dev workflow to locally preview Liquipedia CSS/JS by running a local MITM proxy + file watcher that serves locally-built load.php?only=styles|scripts assets and triggers browser reloads after rebuilds. This fits into the repo’s local development/testing tooling, complementing the existing proxy-based approaches.
Changes:
- Add
scripts/dev-preview/Node tooling: proxy interception, request classification, PAC generation, browser launch, rebuild polling, and file watching/rebuilding. - Add
npm run deventry point plus required dev dependencies. - Document the workflow in
README.mdand ignore the generated.dev-previewworking directory.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/dev-preview/watch.js | Watches local sources and triggers CSS/JS rebuilds, flagging the proxy to reload the browser. |
| scripts/dev-preview/refresh-client.js | Injected client snippet that polls the proxy for rebuild status and reloads the page. |
| scripts/dev-preview/proxy.js | MITM proxy that serves local CSS/JS in place of load.php assets and exposes a rebuild-check endpoint. |
| scripts/dev-preview/pac.js | Generates a PAC script/data URL to scope proxying to liquipedia.net domains. |
| scripts/dev-preview/index.js | Orchestrates initial build, proxy, watcher, and launching a dedicated browser profile. |
| scripts/dev-preview/constants.js | Central constants for paths, ports, watch globs, and browser detection candidates. |
| scripts/dev-preview/classify.js | Classifies load.php requests into styles/scripts vs passthrough. |
| scripts/dev-preview/browser.js | Browser binary detection and launch wrapper for the dev profile + proxy flags. |
| README.md | Adds documentation for the local asset preview workflow and environment overrides. |
| package.json | Adds dev script and new devDependencies needed by the preview tooling. |
| .gitignore | Ignores the .dev-preview runtime directory created by the tool. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+35
to
+59
| let timer = null; | ||
| let pending = { css: false, js: false }; | ||
|
|
||
| function schedule( kind ) { | ||
| pending[ kind ] = true; | ||
| if ( timer ) { | ||
| clearTimeout( timer ); | ||
| } | ||
| timer = setTimeout( async () => { | ||
| timer = null; | ||
| const todo = pending; | ||
| pending = { css: false, js: false }; | ||
| let built = false; | ||
| if ( todo.css && await runBuild( 'build:css' ) ) { | ||
| built = true; | ||
| } | ||
| if ( todo.js && await runBuild( 'build:js' ) ) { | ||
| built = true; | ||
| } | ||
| if ( built ) { | ||
| state.needsReload = true; | ||
| console.log( '[watch] rebuilt — browser will reload' ); | ||
| } | ||
| }, 300 ); | ||
| } |
Comment on lines
+21
to
+22
| // Thin shell: spawn the browser detached-but-tracked so we can kill it on teardown. | ||
| // --disable-quic is required: HTTP/3 bypasses the HTTP proxy entirely. |
The 300ms debounce only coalesces events that arrive before the timer fires. Once it fired, a save landing during the build started a second one: build:css takes ~885ms, so measuring three saves 600ms apart gave overlapping runs (START #2 at t+2384ms against END #1 at t+2730ms) with two `sass` processes writing lua/output/css/main.css at once. The loser's bytes can end up on disk while the marker announces a rebuild, so the browser hot-swaps to stale CSS. writeMarker was already atomic; the asset write was not. Builds now run single-file, and anything changed mid-build stays pending and is picked up when the current one finishes. Same three saves produce two strictly sequential builds instead of three overlapping ones. Also correct the launchBrowser comment: spawn() never passed detached, and it should not — staying in our process group is what lets Ctrl-C reach the browser, with the explicit kill and exit handler in index.js as backup.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Prototype to simplify the proxy setup to one npm command and no need to set up certs. With limited testing, seems to work nicely. Not that battle-tested yet, but wanted to share if you want to try it out while I'm out for the next couple of weeks!
Summary
scripts/dev-preview/and annpm run devcommand: a local MITM proxy serves your locally-built CSS/JS in place of production assets on*.liquipedia.net/commons/load.php, a file watcher rebuilds on.scss/.jssave, and a dedicated browser auto-reloads — no editor integration required.http-mitm-proxy+chokidar), no Python/venv. Proxying is PAC-scoped to*.liquipedia.netand runs in a throwaway browser profile that ignores cert errors, so normal browsing is untouched.How did you test this change?
Limited local testing:
npm run devopens the browser with local assets, and editing.scss/.jsauto-reloads the page. Not extensively battle-tested yet.